Fix 500 when editing an event whose course is in another organization - #2234
Closed
moveson wants to merge 1 commit into
Closed
Fix 500 when editing an event whose course is in another organization#2234moveson wants to merge 1 commit into
moveson wants to merge 1 commit into
Conversation
The event form's course selector only offered the event group organization's courses. An event whose course belongs to a different organization booted the selector to "Create a new course", the setup Stimulus controller copied that blank value into the hidden course_id field, and submitting raised ArgumentError in ChangeEventCourse (a 500) because conform_changed_course runs before_validation, ahead of the belongs_to presence check. Include the event's own course in the selector options when it is missing from the organization's list, and bail out of the course-change conforming when the course is blank so the presence validation returns a normal 422 instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Closing unmerged in favor of fixing the data. The production audit (#2235) found zero cross-organization events, so this 500 cannot occur against production data — the failing case was a staging-only relic from the era when courses had no organization owner, and those rows are being repaired per the remediation plan on #2235. The two useful ideas here move to the follow-up hardening PR proposed on #2235:
The form-side |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the staging 500 hit while testing #2233 (Scout error group 124190 on ost-staging:
ArgumentError: change_event_course must include new_coursefromPATCH /event_groups/2023-marathon-test/events/2023-marathon-test-100k).The bug predates the projections work — the new checkbox was just the first reason anyone submitted the edit form for that event. The chain:
EventSetupPresenter#courses_for_selectoffers only the event group organization's courses, and this event's course belongs to a different organization, so the selector had no option matching the event'scourse_idand booted showing "Create a new course" (value blank).course_idfield on connect, blanking it before the user touches anything.course_idis blank,conform_changed_coursefires as abefore_validation— ahead of thebelongs_to :coursepresence validation — andChangeEventCourse.perform!raises.Two-layer fix:
courses_for_selectnow prepends the event's own course when it is missing from the organization's list, so the selector always reflects reality and the hidden field is never silently blanked.conform_changed_coursereturns early whencourseis nil, letting the presence validation reject the record with a normal 422 re-render instead of a 500 — covering any other path that blankscourse_id.Testing
EventSetupPresenterspec: options include the event's cross-organization course (fails pre-fix); ordinary org courses still listed with the create-new option first.Eventspec: clearingcourse_idon a persisted event is invalid with "must exist" rather than raising (fails pre-fix).🤖 Generated with Claude Code